fix(orchestrator): create /dev/fuse for dockerd; runner parity refinements - #152
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
@macroscope review |
|
Manual reviews triggered for commit All prior checks · these links stay valid even if you push more commits. |
|
Just FYI for future @mentions, I'm Review triggered. The Approvability and Correctness checks are running now and will post results when complete. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
2392540 to
c5533c8
Compare
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| if [ -n \"$DRIVER\" ]; then \ | ||
| printf '{{\"data-root\":\"{DOCKER_DATA_ROOT}\",\"storage-driver\":\"%s\"}}\\n' \"$DRIVER\" > /etc/docker/daemon.json; \ | ||
| fi; \ |
There was a problem hiding this comment.
🟠 High src/lib.rs:1111
When DRIVER is empty, this leaves stale /etc/docker/daemon.json untouched or creates no config, so Docker can retain an unusable storage-driver and use /var/lib/docker instead of /storage/docker. Write data-root unconditionally, and include storage-driver only when DRIVER is forced.
- if [ -n "$DRIVER" ]; then \
- printf '{{"data-root":"{DOCKER_DATA_ROOT}","storage-driver":"%s"}}\\n' "$DRIVER" > /etc/docker/daemon.json; \
- fi; \
+ if [ -n "$DRIVER" ]; then \
+ printf '{{"data-root":"{DOCKER_DATA_ROOT}","storage-driver":"%s"}}\\n' "$DRIVER" > /etc/docker/daemon.json; \
+ else \
+ printf '{{"data-root":"{DOCKER_DATA_ROOT}"}}\\n' > /etc/docker/daemon.json; \
+ fi; \🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @crates/preloop-orchestrator/src/lib.rs around lines 1111-1113:
When `DRIVER` is empty, this leaves stale `/etc/docker/daemon.json` untouched or creates no config, so Docker can retain an unusable `storage-driver` and use `/var/lib/docker` instead of `/storage/docker`. Write `data-root` unconditionally, and include `storage-driver` only when `DRIVER` is forced.
94c62db to
d0215e2
Compare
…ments Follow-ups to the conformance campaign fixes (PR #151): - docker: the krunfw guest kernel has fuse built in, but /dev boots as a plain tmpfs with no device nodes, so fuse-overlayfs (dockerd's fallback when its overlay probe fails) dies with 'fuse: device not found'. The hook now creates /dev/fuse when the kernel lists fuse, letting dockerd auto-pick fuse-overlayfs (CoW) — on this kernel dockerd's overlay2 probe mount gets EINVAL and overlay2 is never viable, so the earlier fix was falling back to vfs. vfs is now forced only when overlay fails AND fuse is absent. - RLIMIT_NOFILE: 524288 instead of 1048576 — systemd's built-in hard default, which is what GitHub's runner service inherits (exact parity). - guest PATH: cargo bin dir matches the runner user (/home/<user>/.cargo) instead of hardcoded /root/.cargo/bin, which the unprivileged runner cannot stat (nodejs/ci EACCES). - multiarch shim: add libsystemd0:amd64 (valkey's x86_64 tarballs link libsystemd.so.0). Verified live on the golden VM: hook creates /dev/fuse, dockerd 28.0.4 reports Storage Driver: fuse-overlayfs, and hello-world runs. 61 orchestrator tests pass.
c5533c8 to
64d0055
Compare
1 participant
Follow-ups to the conformance campaign fixes (PR #151).
/devboots as a plain tmpfs with no device nodes, so fuse-overlayfs (dockerd's fallback when its overlay probe fails) dies withfuse: device not found. The hook now creates/dev/fusewhen the kernel lists fuse, letting dockerd auto-pick fuse-overlayfs (CoW) — on this kernel dockerd's overlay2 probe mount gets EINVAL and overlay2 is never viable, so the earlier fix was falling back to vfs. vfs is now forced only when overlay fails AND fuse is absent./home/<user>/.cargo) instead of hardcoded/root/.cargo/bin, which the unprivileged runner cannot stat (nodejs/ci EACCES).libsystemd0:amd64(valkey's x86_64 tarballs linklibsystemd.so.0).Verified live on the golden VM: hook creates
/dev/fuse, dockerd 28.0.4 reportsStorage Driver: fuse-overlayfs, andhello-worldruns. 61 orchestrator tests pass.Summary by cubic
Creates
/dev/fusein krun guests sodockerdcan auto-selectfuse-overlayfsand keep copy-on-write. Previously we forcedvfswhen theoverlay2probe failed; now we only forcevfsif overlay is unusable and/dev/fuseis missing, and we writedaemon.jsononly when a driver is forced./dev/fusewhen the kernel supports fuse; leaveDRIVERunset when theoverlay2probe succeeds or fails but fuse exists; forceDRIVER=vfsonly when the probe fails and fuse is absent.data-rootwithout forcing a driver, it will no longer be written; configure it elsewhere.RLIMIT_NOFILE=524288, add/home/<user>/.cargoto PATH, and installlibsystemd0:amd64for the multiarch shim.Written for commit 64d0055. Summary will update on new commits.
Note
Create
/dev/fusefordockerdand stop forcingoverlay2/vfsstorage drivers/dev/fuse(viamknod c 10 229) when/proc/filesystemsreports fuse support and the node is missing, indocker_start_commandin lib.rsoverlay2; leavesDRIVERempty so dockerd selects its own drivervfswhen/dev/fuseis absent; otherwise leavesDRIVERempty to allow dockerd's fallback/etc/docker/daemon.json(includingdata-rootandstorage-driver) only whenDRIVERis non-emptydata-rootis no longer written todaemon.jsonwhen no explicit driver is selected; any setup relying on a custom data-root without a forced driver will lose that configuration📊 Macroscope summarized 64d0055. 1 file reviewed, 2 issues evaluated, 0 issues filtered, 1 comment posted
🗂️ Filtered Issues